-
-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Post Templates #1151
base: develop
Are you sure you want to change the base?
Post Templates #1151
Conversation
Also fixes minimum title validation
The line deleted wasn't actually doing anything. Modals with backgrounds are already fixed and we hadn't used modals without backgrounds. This broke non-backgrounded modals however.
Code style: interpolate instead of concatenate Co-authored-by: ArtOfCode <[email protected]>
More generally: I'm not sure that having a Templates category at the top of every page on the site is right - it almost feels too visible for something that won't be used that frequently. I like having it as a category, because it makes template admin familiar, but can we find a way to hide the Templates category from the top bar? Might need to add a field to the Category model. It'll still be linked from /categories, just not on every page. |
You can now remove the category from the header by setting the |
I don't know how this dumb mistake hadn't broken it. The post type for templates should reference a post type, not a post
In terms of functionality, everything should be there, but the UI is really ugly; if anyone has any ideas how to improve it, please let me know. Templates are a markdown tool which lists each template for the post type; clicking on the name will fill in the template, clicking on the info button will show the second modal with information about the template, including a link to the template post. |
Can you position the modal over the post editor (buttons + textbox), read-only with the "apply" action being more obvious? What happens to text that's already in the editor when you apply a template? Does the template get inserted after (or before), are templates not available if there's text, or does existing text get clobbered? (Any but the last is ok with me, preferring an additive approach over disabling.) |
Template content is inserted, not replaced (though apparently I forgot to push that change before going on my break from QPixel, whoops) |
Some existing categories don't set a value. Consider |
Was allowing existing categories to not have a value intentional behavior before this PR? If so, what was the expected behavior? I had assumed that categories not being seeded with values was a bug. |
I believe the default is to show them in order of creation, and until a community needs something beyond Q&A and Meta, there was no reason to set explicit values. (Even then, IIRC correctly, some just set Meta to 99 and otherwise leave them blank.) For intent we'll have to ask @ArtOfCode- ; I'm just reporting what's in production. I don't have an objection to requiring categories to specify where they are in the sequence, but we'll need to add migrations for ones that don't have it set today. |
It was intentional to allow categories to not set the |
Ah, OK. If having it blank had well defined behavior, I'm fine with using -1 instead |
It seems impossible to select the templates post type as listed for a category since the old value remains in the redis cache forever. This may be a problem as editing the "Templates" category might clear templates from the listed values, as it was not a selectable value. Perhaps we need to do something with seeds or after_create handlers to clear specific cache values and make things more robust to changes. Of course, post_types almost never change, but having behavior in the code to automatically deal with necessary cache invalidation for it should a new one be created would be useful. Not really something for this PR though. |
Since the title name of the template is important (it is shown to the user in the selection dropdown), it could be common to have duplicate names for different post types, e.g. "Default" both for Question and for Answer. However, after creation you cannot see in the list what post type it was for. I suggest to add to <%= post.post_type.is_top_level ? post.title : post.parent.title %>
<%= post.post_type.is_closeable && post.closed && !post.duplicate_post ? "[closed]" : "" %>
<%= post.duplicate_post && post.post_type.is_closeable && post.closed ? "[duplicate]" : "" %>
+ <%= post.post_type.name == 'PostTemplate' ? "[#{post.template_post_type.name}]" : "" %>
<% end %>
</div>
<% if (SiteSetting['PostBodyListTruncateLength'] || 0) > 0 %> Which will show up as follows in the list: |
const $postField = $('.js-post-field'); | ||
$tgt = $(ev.target); | ||
const content = $(`#template-md-${$tgt.attr('data-template-id')}`).val(); | ||
replaceSelection($postField, content); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it is intentional that templates are overwriting the selection, or if nothing is selected, just append at the cursor location?
I'm more used to implementations where templates replace the current content. What was the idea behind the different approach? To allow defining sort of common elements as templates?
I can see it happening that a user tries to select text and then accidentally clicks on a template, after which their selection is gone (no confirmation). Not sure if that weighs up against the extra effort of having to confirm something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Monica's comment here: #1151 (comment), but also a confirmation would also be nice to have if they still have selected text that might be overridden (undo functionality is broken on all Markdown tools (#1152), so there's no going back).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah in my tests if you select text it does override (which is luckily consistent with what the javascript method is named after). The question is how complex we would want to make it.
Proof of concept that I made in an afternoon lol
Addresses https://meta.codidact.com/posts/276900
Templates are tied to a post type, so you can have templates for questions, answers, articles, help docs, and even templates for templates (probably not useful, but hey, why not).
Database changes:
posts
,template_post_type_id
before_template_post_type_id
andafter_template_post_type_id
topost_histories
Seed changes
PostType
,PostTemplate
Behavior changes
Other
Trust Level only affects creating posts afaik, so users with the edit ability will still be able to edit these posts since by default, I let everyone be able to view the category. This is easily configured since it's just a normal category, so if you like, you can set the permissions however you want, and even lower it to let normal users create templates.